Skip to content

start needs to be at least a configurable number of hours in the future (default 24)#5815

Merged
dorinhogea merged 1 commit intobloomberg:mainfrom
dorinhogea:block2oldretro
Mar 16, 2026
Merged

start needs to be at least a configurable number of hours in the future (default 24)#5815
dorinhogea merged 1 commit intobloomberg:mainfrom
dorinhogea:block2oldretro

Conversation

@dorinhogea
Copy link
Copy Markdown
Contributor

This is simply to prevent the case when clients choose a start time for retroactive time partition in the past. Start is the next rollout, not the first rollout.

@dorinhogea
Copy link
Copy Markdown
Contributor Author

for #5784

Copy link
Copy Markdown

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
sc_resume_logicalsc_generated
sc_swapfields
consumer_non_atomic_default_consumer_generated
remsql_locks_rte_connect_generated
remsql_locks
sc_downgrade
reco-ddlk-sql

@riverszhang89
Copy link
Copy Markdown
Contributor

@dorinhogea Thank you!

@riverszhang89
Copy link
Copy Markdown
Contributor

How about making it a day in advance (since the smallest granularity is daily). Otherwise if the schema change takes longer than a hour, the current shard will have date which would've belonged to the next rollout. Changing it to a day makes it a lot less likely to happen.

@dorinhogea
Copy link
Copy Markdown
Contributor Author

There is no good value here; we had collapses that take days. Less then 24 hours let use actually test the collapse feature (since daily is the fastest possible rollout). The retroactive partitioning is not meant to be surgical; too new rows will go in the shard 0, too old rows go to shard 1.

@dorinhogea
Copy link
Copy Markdown
Contributor Author

when id doubt, make it configurable (defaults to 24 hours)

@dorinhogea dorinhogea changed the title start needs to be at least 1 hour in the future start needs to be at least a configurable number of hours in the future (default 24) Mar 16, 2026
…re (default 24)

Signed-off-by: Dorin Hogea <dhogea@bloomberg.net>
Copy link
Copy Markdown
Contributor

@riverszhang89 riverszhang89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you!

Copy link
Copy Markdown

@roborivers roborivers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cbuild submission: Success ✓.
Regression testing: Success ✓.

The first 10 failing tests are:
sp_snapshot_generated
consumer_non_atomic_default_consumer_generated
sc_transactional_rowlocks_generated
remsql_locks_rte_connect_generated
remsql_locks
reco-ddlk-sql

@dorinhogea
Copy link
Copy Markdown
Contributor Author

@riverszhang89 thank you

@dorinhogea dorinhogea merged commit 45778b3 into bloomberg:main Mar 16, 2026
4 checks passed
@dorinhogea dorinhogea deleted the block2oldretro branch March 16, 2026 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants